feat(bma): pass CDK managed role to BMA session - #2497
nborges-aws wants to merge 1 commit into
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, well-scoped change. The schema addition, CDK output capture + state recording, graceful error for an outdated @aws/agentcore-cdk, and the simplified China-region gate are all tested. client.py correctly resolves the project root (parents[2] from app/<runtime>/client.py lands at the project root), the state-file path matches DEPLOYED_STATE_RELATIVE_PATH, and the removal path (bedrockManagedAgents: false) clearing bmaSession works because the shallow spread drops undefined keys at JSON.stringify time.
A couple of minor things worth being aware of (not blockers):
src/core/project/backends/cdk.ts~L374–402:updateTargetStateclearsbmaSessionbeforedescribeStack, so a transient CloudFormation error (as opposed to missing outputs) will leave the project with no recorded BMA role until the next successful deploy. Acceptable since the user retries, but you could narrow the window by only clearing whenbmaRuntimes.length === 0and otherwise overwriting in the single finalupdateTargetState.- Dropping the tag/policy heuristics in
manager.tsxmeans projects scaffolded by an older CLI that still have theagentcore:template=BedrockManagedAgentstag +bma-acr-policy.jsonbut nobedrockManagedAgents: truewill no longer be blocked from deploying to China and won't get a session role recorded. That seems intentional (the China deploy will fail at CFN time anyway), but worth confirming there's a migration note or that no in-the-wild projects fall into that gap.
Neither needs changes before merging.
26a1bdd to
e862d98
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2497 +/- ##
=========================================
Coverage 97.40% 97.40%
=========================================
Files 642 642
Lines 46865 46905 +40
=========================================
+ Hits 45647 45687 +40
Misses 1218 1218 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Add
bedrockManagedAgentsto the runtime configuration and set it in newly scaffolded BMA projects. After deployment, capture the CDK-managed session role ARN and associated runtime ARNs in deployed state. This PR also updates the BMA client to select and pass the role for its runtime when creating a session. This client no longer creates or modifies the IAM role.Type of Change
Testing
How have you tested the change?
bun run test(3955 pass, 0 fail)npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.